-
Notifications
You must be signed in to change notification settings - Fork 12k
fix get_num_physical_cores() #1436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
had been broken on complex topologies because "cpu cores" in /proc/cpuinfo is per-"physical id"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't test it on a complex topology, but on my 9900k it still defaults to the number of physical cores (8), so this seems right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
siblings.insert(line); | ||
} | ||
} | ||
if (siblings.size() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
if (siblings.size() > 0) { | |
if (!siblings.empty()) { |
Additional context
/usr/include/c++/11/bits/unordered_set.h:298: method 'unordered_set'::empty() defined here
empty() const noexcept
^
// enumerate the set of thread siblings, num entries is num cores | ||
std::unordered_set<std::string> siblings; | ||
for (uint32_t cpu=0; cpu < UINT32_MAX; ++cpu) { | ||
std::ifstream thread_siblings("/sys/devices/system/cpu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be "/sys/devices/system/cpu/cpu" + std::to_string(cpu) + "/topology/thread_siblings")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, do you want to open a PR with fix?
* Fix the Colab PR * 1435: add timeout for vulkaninfo There's a bug in vulkaninfo where it can hang, and this will prevent koboldcpp from starting. This adds a 5 second timeout * restoring colab.ipynb * Formatting --------- Co-authored-by: henk717 <[email protected]>
had been broken on complex topologies because "cpu cores" in /proc/cpuinfo is per-"physical id"